Skip to content

Extract shared SelectProfile helper to eliminate duplicate profile pickers#4647

Open
mihaimitrea-db wants to merge 30 commits intomainfrom
mihaimitrea-db/stack/auth_logout_deduplication
Open

Extract shared SelectProfile helper to eliminate duplicate profile pickers#4647
mihaimitrea-db wants to merge 30 commits intomainfrom
mihaimitrea-db/stack/auth_logout_deduplication

Conversation

@mihaimitrea-db
Copy link
Collaborator

@mihaimitrea-db mihaimitrea-db commented Mar 3, 2026

🥞 Stacked PR

Use this link to review incremental changes.


Four places built nearly identical promptui.Select prompts for interactive profile selection (auth logout, auth token, cmd/root/auth.go, cmd/root/bundle.go). This PR extracts a reusable profile.SelectProfile function that accepts a declarative SelectConfig with label, profiles, and template strings, replacing all four implementations.

Changes

  • Add profile.SelectProfile in libs/databrickscfg/profile/select.go — a shared interactive profile picker that accepts a SelectConfig (label, profiles, template strings) and returns the selected profile name.
  • Replace the four inline promptui.Select implementations in cmd/auth/logout.go, cmd/auth/token.go, cmd/root/auth.go, and cmd/root/bundle.go with calls to SelectProfile.
  • Add AccountID to Profiles.SearchCaseInsensitive so all pickers support searching by account ID, not just name and host.
  • Extract writeConfigFile helper in libs/databrickscfg/ops.go to consolidate the repeated default-comment / backup / save sequence shared by SaveToProfile and DeleteProfile.

Why

The four profile pickers each duplicated the same prompt setup, searcher wiring, and result extraction. This made it easy for behavior to diverge (e.g., only the logout picker searched by account ID). A single shared helper keeps the UX consistent and reduces the surface area for future changes.

Tests

  • Existing unit and acceptance tests for auth logout, auth token, workspace/account profile selection, and bundle profile resolution continue to pass — the refactor is behavior-preserving.
  • The SelectProfile helper is exercised indirectly through all existing callers.

@eng-dev-ecosystem-bot
Copy link
Collaborator

eng-dev-ecosystem-bot commented Mar 3, 2026

Commit: b6b668f

Run: 22722741538

Env 🪲​BUG ❌​FAIL 🟨​KNOWN 🔄​flaky 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
💚​ aws linux 8 7 268 786 8:03
💚​ aws windows 8 7 270 784 6:58
🔄​ aws-ucws linux 1 8 7 364 701 14:07
🔄​ aws-ucws windows 2 7 7 366 699 11:01
🪲​ azure linux 1 8 1 1 9 262 784 145:41
🪲​ azure windows 1 8 1 1 9 264 782 145:45
🪲​ azure-ucws linux 1 7 1 2 1 9 360 697 126:27
🪲​ azure-ucws windows 1 8 1 1 1 9 362 695 130:59
🪲​ gcp linux 1 8 1 1 9 258 787 146:29
🪲​ gcp windows 1 8 1 1 9 260 785 142:39
26 interesting tests: 8 FAIL, 7 SKIP, 7 RECOVERED, 2 flaky, 1 KNOWN, 1 BUG
Test Name aws linux aws windows aws-ucws linux aws-ucws windows azure linux azure windows azure-ucws linux azure-ucws windows gcp linux gcp windows
🟨​ TestAccept 💚​R 💚​R 💚​R 🔄​f 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K
🪲​ TestAccept/bundle/deployment/bind/alert 🙈​s 🙈​s 🙈​s 🙈​s 🪲​B 🪲​B 🪲​B 🪲​B 🪲​B 🪲​B
❌​ TestAccept/bundle/deployment/bind/alert/DATABRICKS_BUNDLE_ENGINE=direct ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/deployment/bind/alert/DATABRICKS_BUNDLE_ENGINE=terraform ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/generate/alert ✅​p ✅​p ✅​p ✅​p ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/generate/alert/DATABRICKS_BUNDLE_ENGINE=direct ✅​p ✅​p ✅​p ✅​p ❌​F ❌​F ✅​p ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/generate/alert/DATABRICKS_BUNDLE_ENGINE=terraform ✅​p ✅​p ✅​p ✅​p ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/resources/alerts/with_file ✅​p ✅​p ✅​p ✅​p ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/resources/alerts/with_file/DATABRICKS_BUNDLE_ENGINE=direct ✅​p ✅​p ✅​p ✅​p ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/resources/alerts/with_file/DATABRICKS_BUNDLE_ENGINE=terraform ✅​p ✅​p ✅​p ✅​p ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
🙈​ TestAccept/bundle/resources/permissions 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions 💚​R 💚​R 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=direct 💚​R 💚​R 💚​R 💚​R
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 💚​R 💚​R 💚​R 💚​R
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions 💚​R 💚​R 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=direct 💚​R 💚​R 💚​R 💚​R
💚​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 💚​R 💚​R 💚​R 💚​R
🙈​ TestAccept/bundle/resources/postgres_branches/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/recreate 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/update_protected 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/without_branch_id 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_endpoints/recreate 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/synced_database_tables/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🔄​ TestAccept/ssh/connect-serverless-gpu 🙈​s 🙈​s ✅​p 🔄​f 🙈​s 🙈​s 🔄​f 🔄​f 🙈​s 🙈​s
💚​ TestAccept/ssh/connection 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
🔄​ TestFetchRepositoryInfoAPI_FromRepo ✅​p ✅​p 🔄​f ✅​p ✅​p ✅​p 🔄​f ✅​p ✅​p ✅​p
Top 23 slowest tests (at least 2 minutes):
duration env testname
7:56 azure-ucws linux TestAccept/bundle/generate/alert/DATABRICKS_BUNDLE_ENGINE=direct
5:29 aws-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
4:15 aws-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
4:11 aws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
4:10 aws-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
4:07 aws-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:45 azure-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:44 aws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:40 azure linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:31 gcp windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:29 aws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:29 gcp linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:25 azure-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:24 gcp windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:21 aws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:17 gcp linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:29 azure-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:23 aws linux TestAccept
2:19 azure linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:15 azure windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:15 azure-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:11 aws-ucws linux TestAccept
2:05 azure windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct

@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from c6d25f1 to c9512bf Compare March 4, 2026 09:06
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from 8a5d258 to b40887a Compare March 4, 2026 09:45
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from b40887a to 16de011 Compare March 4, 2026 10:25
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from 16de011 to 3116cbb Compare March 5, 2026 12:39
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from 3116cbb to 634b7b9 Compare March 5, 2026 12:52
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from 634b7b9 to 40c98ed Compare March 5, 2026 13:06
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from 40c98ed to 76e7b0e Compare March 5, 2026 13:15
@mihaimitrea-db mihaimitrea-db changed the base branch from main to mihaimitrea-db/stack/auth_logout_profile_picker March 5, 2026 13:30
Implement the initial version of databricks auth logout which removes a profile from ~/.databrickscfg and clears associated OAuth tokens from the token cache.

This iteration supports explicit profile selection via --profile and a --force flag to skip the confirmation prompt. Interactive profile selection will be added in a follow-up.

Token cache cleanup is best-effort: the profile-keyed token is always removed, and the host-keyed token is removed only when no other profile references the same host.
Replace plain fmt.Sprintf confirmation prompt with a structured template using cmdio.RenderWithTemplate. The warning now uses color and bold formatting to clearly highlight the profile name, config path, and consequences before prompting for confirmation.
Resolve config path from the profiler instead of hardcoding fallbacks. Delete the profile before clearing the token cache so a config write failure does not leave tokens removed. Fix token cleanup for account and unified profiles by computing the correct OIDC cache key (host/oidc/accounts/<account_id>). Drop the nil profiler guard, add a success message on logout, and extract backupConfigFile in ops.go to remove duplication. Consolidate token cleanup tests into a table-driven test covering shared hosts, unique hosts, account, and unified profiles.
Merge shared-host token deletion verification into one main parametrized test by addding the hostBasedKey and isSharedKey parameters to each case. This replaces the TestLogoutTokenCacheCleanup test with an assertion: host-based keys are preserved when another profile shares the same host, and deleted otherwise.
Rewrite the test to use inline config seeds and explicit expected state. Add cases for deleting the last non-default profile, deleting a unified host profile with multiple keys, and deleting the DEFAULT section.
- Use profiler.GetPath() to resolve config path instead of hardcoding platform-specific defaults for the help text.
- Read DATABRICKS_CONFIG_FILE via env.Get(ctx, ...) instead of os.Getenv to respect context-level env overrides.
- Add abort message when user declines the confirmation prompt.
- Guard DeleteProfile against non-existent profiles to avoid creating unnecessary backup files.
- Add TestDeleteProfile_NotFound for the error path.
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from 336ab51 to 02e7989 Compare March 5, 2026 14:19
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from de0d0ee to 16a1df7 Compare March 5, 2026 14:29
When --profile is not specified in an interactive terminal, show a
searchable prompt listing all configured profiles. Profiles are sorted
alphabetically and displayed with their host or account ID. The picker
supports fuzzy search by name, host, or account ID.
Document the four interaction modes (explicit profile, interactive
picker, non-interactive error, and --force) in the command's long
help text.
…ckers

Four places built nearly identical promptui.Select prompts for
interactive profile selection (auth logout, auth token, root auth,
root bundle). Extract a reusable profile.SelectProfile function that
takes a declarative SelectConfig with label, profiles, and template
strings.

Also: add AccountID to Profiles.SearchCaseInsensitive so all pickers
support searching by account ID, and remove the redundant
not-found guard in DeleteProfile (ini.DeleteSection is already a
no-op for missing sections).
Extract writeConfigFile helper to consolidate the repeated
default-comment, backup, and save sequence shared by SaveToProfile
and DeleteProfile.

Also switch auth login from os.Getenv to env.Get(ctx, ...) for
DATABRICKS_CONFIG_FILE so it respects context-level env overrides,
consistent with the rest of the codebase.
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_deduplication branch from 16a1df7 to b6b668f Compare March 5, 2026 14:32
@mihaimitrea-db mihaimitrea-db force-pushed the mihaimitrea-db/stack/auth_logout_profile_picker branch from 369f620 to 4bd2e44 Compare March 5, 2026 14:32
@mihaimitrea-db mihaimitrea-db marked this pull request as ready for review March 5, 2026 14:59
@mihaimitrea-db mihaimitrea-db changed the base branch from mihaimitrea-db/stack/auth_logout_profile_picker to main March 5, 2026 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants